Skip to content

docs: sync env var docs with what the code reads#96

Open
turbomam wants to merge 6 commits into
mainfrom
readme-env-var-sync
Open

docs: sync env var docs with what the code reads#96
turbomam wants to merge 6 commits into
mainfrom
readme-env-var-sync

Conversation

@turbomam

@turbomam turbomam commented Jul 7, 2026

Copy link
Copy Markdown
Member

Closes #95.

Brings the README, .env.example, and the code into agreement. The code reads env vars only through os.environ; after this change, every variable it reads is documented and has a placeholder, and nothing it never reads appears in either file.

  • Fix cp .env-example .env to .env.example (the wrong name made the first quickstart step fail).
  • Remove DEFAULT_MODEL, MAX_TOKENS, TEMPERATURE from the README Configuration section (no code reads them).
  • Remove the commented OPENALEX_API_KEY and UNPAYWALL_EMAIL from .env.example (no code reads them, and there is no Unpaywall code despite the "required" comment). If either is planned, say so and I will restore it.
  • Document CONTACT_EMAIL and CLOUD_ML_REGION.
  • Add the ingestion-tuning vars (NMDC_EDI_MAX_XML_CHARS, NMDC_DATAONE_SOLR_MAX_XML_CHARS, NMDC_HTTP_RETRY_ATTEMPTS, NMDC_HTTP_RETRY_BACKOFF_SECONDS, NMDC_HTTP_MAX_RETRY_DELAY_SECONDS, NMDC_HTTP_POOL_CONNECTIONS, NMDC_HTTP_POOL_MAXSIZE) to both, with their code defaults.
  • Fix a README example that pointed at a non-existent tests/test_example.py.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Make README, .env.example, and the code agree: every variable the code
reads via os.environ is documented and has a placeholder, and nothing
unread appears in either.

- fix cp .env-example -> .env.example (broke the first quickstart step)
- drop DEFAULT_MODEL/MAX_TOKENS/TEMPERATURE from README (no code reads them)
- drop OPENALEX_API_KEY/UNPAYWALL_EMAIL from .env.example (no code reads them)
- document CONTACT_EMAIL and CLOUD_ML_REGION
- add the NMDC_* ingestion-tuning vars to README and .env.example
- fix a non-existent test path in the README

Closes #95

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 7, 2026 13:52
@turbomam turbomam requested a review from hesspnnl July 7, 2026 13:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates project documentation and the sample .env file so they reflect the environment variables the Python code actually reads (via os.environ.get(...)), and fixes a couple of quickstart/testing instructions that were pointing at the wrong filenames.

Changes:

  • Fix Quick Start copy command to use .env.example.
  • Update README configuration docs to remove unused variables and document the env vars that are actually read (including ingestion-tuning variables and defaults).
  • Update .env.example to include the ingestion-tuning variables and remove placeholders for env vars not read by the code.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
README.md Aligns documented env vars and examples with current runtime behavior and existing test files.
.env.example Adds placeholders for env vars read by code (ingestion/HTTP tuning) and removes ones that aren’t used.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md Outdated
turbomam and others added 2 commits July 7, 2026 14:37
CLOUD_ML_REGION and GEMINI_REGION shipped as the literal placeholder
'region'. GEMINI_REGION is read first and only falls back to CLOUD_ML_REGION
when unset, so a copied .env with GEMINI_REGION=region sends 'region' to
Vertex and gets a confusing 404. Ship the real default (us-east5) and correct
the comment (it takes precedence when set, it does not default to
CLOUD_ML_REGION unless unset).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread README.md
- `GOOGLE_APPLICATION_CREDENTIALS`: Path to a GCP service account JSON file (for Vertex AI).
- `VERTEX_PROJECT_ID`: (Optional) GCP project id for Vertex. If not provided, the SDK will attempt to infer it from credentials.
- `GEMINI_REGION`: (Optional) GCP region for Gemini/Vertex (defaults to `us-east5` or `CLOUD_ML_REGION`).
- `GEMINI_REGION`: (Optional) Vertex region override for Gemini calls (falls back to `CLOUD_ML_REGION`, then `us-east5`).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caufieldjh @turbomam I did not notice this before, but is there a reason there are so many places to declare gcp region? can we change this to just GCP_REGION ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with all things GCP, the answer is "yes and no". I think the values can be different (https://docs.cloud.google.com/gemini-enterprise-agent-platform/resources/locations kinda gets into this but doesn't fully answer the question).
For our purposes I think it's enough just to set the project ID and region.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I am asking because in this case they are filling the same value... to me this indicates they are not actually different. At least how we are using them

@hesspnnl hesspnnl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good - thanks for the initiative on this. I have one concern I commented above. I made some further docs changes that I had been wanting to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

README and .env.example are out of sync with the environment variables the code reads

4 participants